add commands for warming the overlay-base cache #4195
Conversation
ginsbach
force-pushed
the
ginsbach/WarmDatabaseUnderlay
branch
9 times, most recently
from
d83409d to
4d6c60c
Compare
ginsbach
changed the title
ginsbach
force-pushed
the
ginsbach/WarmDatabaseUnderlay
branch
4 times, most recently
from
9073228 to
daad02a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to warm the overlay-base cache by introducing a second query server configured with specific cache-warming flags. The implementation adds new commands for cache warming operations and manages database registration between the two query servers.
Key changes:
- Adds a second query server instance dedicated to cache warming with flags
--no-evaluate-as-overlay,--cache-at-frontier, and--warm-cache-only - Implements three new commands for warming overlay-base cache: for single queries, multiple queries, and query suites
- Introduces database registration switching logic to ensure only one query server can register a database at a time
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| local-databases.test.ts | Adds test coverage for database registration switching between query servers |
| query-server-client.ts | Adds cache warming configuration flags and descriptive logging for the warming query server |
| local-query-run.ts | Conditionally skips showing results when warming cache |
| local-queries.ts | Implements cache warming commands and routes them to the appropriate query server |
| ast-cfg-commands.ts | Updates function call to include new warmOverlayBaseCache parameter |
| extension.ts | Initializes the second query server and wires up restart logic |
| database-manager.ts | Implements database registration switching between query servers |
| loggers.ts | Adds dedicated logger for the cache warming query server |
| commands.ts | Defines type signatures for new cache warming commands |
| package.json | Registers new commands in VS Code command palette and context menus |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ginsbach
force-pushed
the
ginsbach/WarmDatabaseUnderlay
branch
from
daad02a to
f903a5d
Compare
nickrolfe
left a comment
nickrolfe
left a comment
There was a problem hiding this comment.
The overall approach makes sense, and adding a second query-server was less invasive than I feared, but I have a few suggestions.
| { | ||
| title: "Running query", | ||
| title: warmOverlayBaseCache | ||
| ? "Warm overlay-base cache for query" |
There was a problem hiding this comment.
How about removing "for query", to shorten it slightly?
There was a problem hiding this comment.
I'm not strictly against this, but on first impression, I think that "for query" actually does carry information here.
The cache will be set up so that this individual query will afterwards be evaluated incrementally, but that only holds for that one query.
Also, I don't think the command name is overly long.
| if (this.warmOverlayBaseCache) { | ||
| args.push( | ||
| "--no-evaluate-as-overlay", | ||
| "--cache-at-frontier", | ||
| "--warm-cache-only", | ||
| ); | ||
| } | ||
|
|
||
| const queryServerSuffix = this.warmOverlayBaseCache | ||
| ? " for warming overlay-base cache" | ||
| : ""; | ||
|
|
||
| const child = spawnServer( | ||
| this.config.codeQlPath, | ||
| "CodeQL query server", | ||
| `CodeQL query server${queryServerSuffix}`, |
There was a problem hiding this comment.
I notice that we spawn this second query server on startup. I imagine the vast majority of users will never need it. Can we find a way to start it lazily?
There was a problem hiding this comment.
Commit 7 starts the second query server on demand, and also hides the logger until needed.
Co-authored-by: Nick Rolfe <nickrolfe@github.com>
Co-authored-by: Nick Rolfe <nickrolfe@github.com>
Co-authored-by: Nick Rolfe <nickrolfe@github.com>
ginsbach
force-pushed
the
ginsbach/WarmDatabaseUnderlay
branch
from
b9c1afd to
9947c9d
Compare
ginsbach
force-pushed
the
ginsbach/WarmDatabaseUnderlay
branch
from
9947c9d to
56f8652
Compare
This PR adds a second query server that is configured with
and several commands:
These commands are based on
but they run on the second query server and their results are not loaded into the "CodeQL Query Results" view.
One complication of this approach with two query servers is that only one of them can register the database at any given time. This is handled via
withDatabaseInQsForWarmingOverlayBaseCacheand seems to work well in practice. It does mean that running a query while a cache warming is ongoing will result in an error message. That seems better than silently running on the wrong kind of query server, though, which would be the effect on an alternative implementation with a single query server that gets killed and restarted.